Skip to content

searcher: fix cache size env vars rendering 0 for non-Gi storageSize - #948

Merged
marcleblanc2 merged 2 commits into
mainfrom
searcher-storagesize-g-units
Oct 19, 2026
Merged

marcleblanc2 merged 2 commits into
mainfrom
searcher-storagesize-g-units

Conversation

@marcleblanc2

@marcleblanc2 marcleblanc2 commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Fixes the issue for impacted customers without requiring them to change their deployment, beyond upgrading.

Problem

SEARCHER_CACHE_SIZE_MB and SYMBOLS_CACHE_SIZE_MB were derived with trimSuffix "Gi" .Values.searcher.storageSize | mul 450. Any other valid PVC size (150G, 1Ti, 500000M) left a non-numeric string that sprig cast to 0, so both env vars rendered "0".

searcher treats an explicit 0 as "never evict" (an unset variable auto-sizes instead), so both caches grew unbounded until the PVC filled and the pod failed to start:

failed to setup TMPDIR: mkdir /mnt/cache/searcher-0/.searcher.tmp/tmpfriend-...: no space left on device

Reported by a customer with searcher.storageSize: 150G after upgrading 6.5.0 → 7.2.0. The template has behaved this way since v6.4 (#677), so the disk had been filling for months; it was only noticed in the logs after the upgrade.

Fix

  • Extracted the Kubernetes quantity parser that sourcegraph.redis.maxmemory already had into a shared sourcegraph.quantityBytes helper in templates/_helpers.tpl. Accepts any quantity suffix (k/M/G/T/P/E, their i binary forms, fractional values, suffixless bytes).
  • New sourcegraph.searcher.cacheSizeMB helper in templates/_helpers/_searcherCacheSize.tpl uses it to compute floor(bytes / 1e6) * 45 / 100, the same arithmetic searcher uses when it auto-sizes from the volume.
  • The render fails, with a pointer to searcher.autoCacheSize=true, when the value is unparseable or so small the result would be 0 (anything under 3M). Silently disabling eviction is no longer possible.
  • Both env blocks in searcher.StatefulSet.yaml call the helper. values.yaml comment, README (helm-docs), CHANGELOG, and a new searcherCacheSize_test.yaml suite.

Behaviour change for existing deployments

Binary units are now converted exactly instead of being treated as decimal, so every Gi deployment renders a slightly higher value and searcher pods restart on upgrade. The default 26Gi goes from 11700 to 12562; 100Gi from 45000 to 48318. These match what searcher's auto-sizing would pick for the same disk.

Two caches at 45% each is 90% of the nominal PVC. The old Gi × 450 math happened to leave more slack (2 × 11700 MB is 84% of 26Gi). searcher's auto mode uses 90% of the mounted filesystem, which is a few percent smaller than the PVC request, and TMPDIR shares the volume. Operators who want more headroom should enable searcher.autoCacheSize.

Verification

./scripts/helm-docs.sh             # no diff
helm unittest charts/sourcegraph   # 32 suites, 195 tests passed
helm lint charts/sourcegraph
searcher.storageSize before after
(default 26Gi) 11700 12562
100Gi 45000 48318
150G 0 (no eviction) 67500
1Ti 0 (no eviction) 494779
1.5G 0 (no eviction) 675
500000M 0 (no eviction) 225000
6442450944 (bytes) 0 (no eviction) 2898
3M 0 (no eviction) 1
2M, 0G 0 (no eviction) render fails
1Tb 0 (no eviction) render fails
1Tb + autoCacheSize: true env vars omitted env vars omitted

SEARCHER_CACHE_SIZE_MB and SYMBOLS_CACHE_SIZE_MB were derived with
trimSuffix "Gi" | mul 450, so a storageSize like 150G left a non-numeric
string that sprig cast to 0. searcher treats 0 as "never evict", so both
caches grew unbounded until the PVC filled and the pod failed to start with
"failed to setup TMPDIR: ... no space left on device".

Move the math into a sourcegraph.searcher.cacheSizeMB helper in its own
file under templates/_helpers/. It accepts a whole number of M, Mi, G or Gi
and fails the render on anything else, unless searcher.autoCacheSize is
enabled (the env vars are omitted in that case).

Co-authored-by: Amp <amp@ampcode.com>
Amp-Thread-ID: https://ampcode.com/threads/T-01a146da-9505-715c-949c-1146cc4e7e33
@marcleblanc2
marcleblanc2 force-pushed the searcher-storagesize-g-units branch from c3ca70d to e203311 Compare October 16, 2026 22:39
@marcleblanc2 marcleblanc2 changed the title searcher: accept storageSize in G as well as Gi for cache sizing searcher: accept storageSize in M, Mi, G and Gi for cache sizing Oct 16, 2026
@github-actions

github-actions Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

@marcleblanc2
marcleblanc2 requested a review from a team October 16, 2026 23:11
@DaedalusG

Copy link
Copy Markdown
Contributor

I don't mind this, but it seems like it might be a little auto magic for the style we typically take with our manifests. I'd like to get a second opinion from someone else on the team, wdyt @filiphaftek ?

…arser (#965)

Follow-up to #948, targeting its branch so it can be merged into the
same fix. #948's allowlist (`M|Mi|G|Gi`) turns `1Ti` and other valid PVC
sizes from "silently renders `0`" into "`helm upgrade` refuses", and it
can still render `0` for `0G`/`1M`/`2M`, which searcher treats as "never
evict".

This extracts the Kubernetes quantity parser that
`sourcegraph.redis.maxmemory` already had into a shared
`sourcegraph.quantityBytes` helper and uses it for
`searcher.storageSize`, so any quantity (`1Ti`, `1.5G`, `1024Ki`,
suffixless bytes) works and the parser is defined once. The 45% is
computed the way searcher auto-sizes (`floor(bytes/1e6) * 45 / 100`)
instead of treating `Gi` as `G`, and the render fails when the result
would be `0`. Behaviour change to be aware of: existing `Gi` deployments
get a slightly higher value on upgrade (default `26Gi`: `11700` →
`12562`), called out in the CHANGELOG.

## Test plan

`helm unittest charts/sourcegraph` (195 passed, incl. the unchanged
Redis `maxmemory` suite covering the extracted parser), `helm lint`,
`./scripts/helm-docs.sh` clean.

| `searcher.storageSize` | `main` | #948 | this PR |
|---|---|---|---|
| `26Gi` (default) | `11700` | `11700` | `12562` |
| `150G` | `0` | `67500` | `67500` |
| `1Ti` | `0` | render fails | `494779` |
| `1.5G` | `0` | render fails | `675` |
| `2M` / `0G` | `0` | `0` | render fails |
| `1Tb` | `0` | render fails | render fails |
| `1Tb` + `autoCacheSize: true` | omitted | omitted | omitted |

Co-authored-by: Amp <amp@ampcode.com>
@marcleblanc2 marcleblanc2 changed the title searcher: accept storageSize in M, Mi, G and Gi for cache sizing searcher: fix cache size env vars rendering 0 for non-Gi storageSize Oct 19, 2026
@marcleblanc2
marcleblanc2 enabled auto-merge (squash) October 19, 2026 20:34
@marcleblanc2
marcleblanc2 disabled auto-merge October 19, 2026 20:35
@marcleblanc2
marcleblanc2 merged commit bbe8587 into main Oct 19, 2026
8 checks passed
@marcleblanc2
marcleblanc2 deleted the searcher-storagesize-g-units branch October 19, 2026 20:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants